Add API spec for logger client integration into cdp-client#25
Add API spec for logger client integration into cdp-client#25stefanrammo wants to merge 1 commit intomainfrom
Conversation
|
"Uses ServerPort tree navigation for discovery (works with all CDP versions). Service-based discovery and proxy transport via ServicesNotification are documented as future work for CDP 5.1+." The reason CDP Studio moved to service-based discovery is that this tree scanning to find loggers and the ServerPort property caused performance issues for larger projects. Also, this here is a new feature, so I think it is fine if it only works with CDP 5.1+. People can use the existing logger client for old versions if needed. All in all, in my opinion cdp-client logger support should only use service-based discovery without any tree scanning. |
spec-logger-integration.md
Outdated
| }); | ||
| ``` | ||
|
|
||
| This requires knowing the `ServerPort` child convention, extracting the hostname, and managing two independent client lifecycles. The logger client (`cdplogger-client`) is a separate npm package with its own protobuf schema and WebSocket connection, making the setup even more cumbersome. |
There was a problem hiding this comment.
Also, one must know "App.CDPLogger" is the path and this must be updated when CDP project is restructured.
spec-logger-integration.md
Outdated
|
|
||
| Instead of opening a direct WebSocket to the logger port, the logger connection could tunnel through the existing StudioAPI WebSocket — the same way sibling app connections work via `connectViaProxy()`. This avoids firewall and port-opening issues when the logger port is not directly reachable from the client. | ||
|
|
||
| Service-based discovery and proxy transport are the preferred approach for CDP 5.1+ and should be added in a follow-up. The `ServerPort` approach is implemented first for backward compatibility with all CDP versions. |
There was a problem hiding this comment.
I would enable proxy transport by default because browser security settings often reject connections to other ports, especially when using https. And I think supporting only CDP 5.1 is fine
There was a problem hiding this comment.
And if it makes implementation easier, I don't think we need to support direct connection. The proxy performance is quite good.
There was a problem hiding this comment.
And one more reason the direct connection is not so useful - the logger server api does not support authentication, so it is recommended to block that port with firewall. This proxy thing allows to serve logged data behind studioapi authentication.
Ok, I now read the rest of the spec and there is no tree scanning. User must hard-code the path, e.g. "App.CDPLogger". Still, I don't think this is useful enough at the moment. If one hard-codes the path to logger, they might as well hard-code the ServerPort. Better to not think about older CDP versions and directly move to implementing service-based discovery. |
|
Something to consider for the future (I think not needed for the first step) is to make it easier to ask historic data for a real-time value. Kind of like CDP Studio Analyze mode auto-discovers if added node has historic data available and lists the history sources when right-clicking on the legend. |
spec-logger-integration.md
Outdated
|
|
||
| ```javascript | ||
| const loggers = await client.loggers(); // returns all discovered logger services | ||
| const logger = loggers[0]; // or find by name/metadata |
There was a problem hiding this comment.
"or find by name/metadata" - yes, as the metadata includes the logger path in the metadata name field, should allow to refer to logger using that as well .
|
Spec reviews should also have 2 reviewers |
stefanrammo
force-pushed
the
spec/logger-integration
branch
from
deda29b to
60f26a9
Compare
| **Behavior:** | ||
|
|
||
| - `client.logger()` finds the first service with `proxy_type === 'logserver'` in `availableServices`, creates a service transport via `makeServiceTransport(serviceId)`, and passes it to the logger client. The logger protocol (`DBMessaging.Protobuf.Container`) is tunneled through the StudioAPI WebSocket via `ServiceMessage` — no direct connection to the logger port. | ||
| - `client.logger(name)` filters by service name when multiple loggers exist. The service name is the logger's node path (e.g., `'App.CDPLogger'`). |
There was a problem hiding this comment.
Should also add a method that returns a list of all loggers. It should return when the first ServicesRequest gets a response (I presume the client sets subscribe to true, so it always has the current list of services after that)
|
|
||
| - `client.logger()` finds the first service with `proxy_type === 'logserver'` in `availableServices`, creates a service transport via `makeServiceTransport(serviceId)`, and passes it to the logger client. The logger protocol (`DBMessaging.Protobuf.Container`) is tunneled through the StudioAPI WebSocket via `ServiceMessage` — no direct connection to the logger port. | ||
| - `client.logger(name)` filters by service name when multiple loggers exist. The service name is the logger's node path (e.g., `'App.CDPLogger'`). | ||
| - Returns `Promise<LoggerClient>`. If no matching logger service has been announced yet, the promise waits until one appears via `ServicesNotification`. |
There was a problem hiding this comment.
If they wait forever, should have an optional timeout argument.
Specifies two changes: - Bundle cdplogger-client into cdp-client as studio.logger - Add client.logger() method using service-based discovery and proxy transport via makeServiceTransport (CDP 5.1+) Logger protocol is tunneled through the StudioAPI WebSocket, serving data behind authentication with no direct connection to the logger port.
stefanrammo
force-pushed
the
spec/logger-integration
branch
from
60f26a9 to
8917d65
Compare
3 participants
Specifies two changes to integrate the CDP Logger Client into the main JavaScript CDP Client package:
Bundle logger client as studio.logger — move the cdplogger-client code into the cdp-client package so users only need one require()
Add client.logger(path) method — auto-discovers the logger's WebSocket endpoint by reading ServerPort and ServerIP from the node's children, then creates and caches a logger client connection
Uses ServerPort tree navigation for discovery (works with all CDP versions). Service-based discovery and proxy transport via ServicesNotification are documented as future work for CDP 5.1+.
Includes 5 use case examples, failure/timeout semantics, caching and lifecycle behavior, and migration notes.